-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try: Block controls rendering by block's edit
#830
Conversation
I'll take a deeper look tomorrow, but here are my first thoughts about this:
I personally always prefer the flexibility and thus avoid the I really like this proposal 👍 I'll take a deeper look tomorrow. |
@aduth Do you have any concern blocking this PR? I'm keen to get this merged and try to use this approach for everything :) |
I'd hoped to solicit some more attention to this pattern, but I'm rather content with it. I'll plan to rebase / evaluate dropping BlockControls / merge tomorrow. Do you think I ought to convert existing usage now or just get the Fill in place? |
Yes, this deserves more attention IMO too. But anyway, let get this merge, I can help with converting existing controls. |
Since it’s known at time of render
e30fddf
to
8c42fdd
Compare
This pull request explores an option to render a block's controls in its
edit
implementation.Benefits:
controls
callbacks have no access to theedit
component instance, so in the case of proper component classes, they cannot make use of component stateedit
isActive
andonClick
callbacks, since it's easy enough to bind useattributes
andsetAttributes
from the bound component instance or rendering scope.Downsides:
focus
to ensure controls are only rendered if the block is currently selectedFor related discussion, see also #717 (comment).
Open questions:
<BlockControls />
as an additional layer when the block implementer could as easily render the<Fill />
directly?Formatting.Controls
slot, do we care to make this the preferred approach if we could support both just the same? Could be nice to consolidate, but only if it's not viewed as more complex.uid
fromVisualEditorBlock
toBlockControls
directly, bypassing the need for the block implementer to concern themselves with whether the block is currently selected, at the cost of reducing transparencyTesting instructions:
Ensure that heading block controls are shown and behave the same as in master. Only the heading block has been ported in this demonstration.